Modularize zkBob pool and add energy redeemer example #78
Conversation
k1rill-fedoseev
force-pushed
the
feat/energy-redeemer
branch
from
c731517 to
3d1f1b3
Compare
| * @param _maxWeeklyAvgTvl max seen average tvl over period of at least 1 week, might not be precise. | ||
| */ | ||
| function initialize( | ||
| uint32 _txCount, |
There was a problem hiding this comment.
Does it make sense to copy this state from the pool contract during initialisation rather than specify them manually?
There was a problem hiding this comment.
I didn't want to include logic for raw storage reads in the main contract, so instead I placed it in the one-time migration contract. I think it's a bit safer and flexible approach overall.
In case we will ever again need to redeploy accounting module (e.g. to change limits logic), such generic initialization logic would be also helpful.
| @@ -329,7 +324,10 @@ abstract contract ZkBobPool is IZkBobPool, EIP1967Admin, Ownable, Parameters, Zk | |||
| */ | |||
| function recordDirectDeposit(address _sender, uint256 _amount) external { | |||
There was a problem hiding this comment.
Does it make sense to call recordOperation from the queue contract directly? What is the goal of having this functionality in the pool contract?
There was a problem hiding this comment.
I think it's better to keep it this way, to reduce dependencies between pool modules. It potentially simplifies future upgrades, as there less address pointers being used between various contracts and permission logic in contract becomes simpler.
k1rill-fedoseev
force-pushed
the
feat/energy-redeemer
branch
from
20623dc to
39254e9
Compare
k1rill-fedoseev
force-pushed
the
feat/energy-redeemer
branch
from
39254e9 to
0d46f96
Compare
This PR simplifies logic of core ZkBobPool contract, by extracting limits, accounting, and KYC logic into an external contract.
Changelog
Changes in the ZkBobPool interface:
zkBobPool.getLimitsFor(...)->zkBobPool.accounting().getLimitsFor(...)zkBobPool.kycProvidersManager()->zkBobPool.accounting().kycProvidersManager()Changes in denomination:
accounting.setLimits(...)now expects values in zkBob units, without denominatorMigration details
There are a few data structures that are being deprecated during the migration:
slot0andslot1. These should be copied to the new module during the migration.maxWeeklyAvgTvlandmaxWeeklyTxCountstats.